[pull] main from databricks:main#36
Merged
Merged
Conversation
…ak (#424) Two complementary fixes for the Thrift + CloudFetch ECONNRESET ("socket hang up") failures when draining large (~100MB) results. 1. Retry transient network errors in HttpRetryPolicy (supersedes/folds in #398, fixes #260). invokeWithRetry now catches operation rejections and classifies them via isRetryableNetworkError (ECONNRESET/ECONNREFUSED/ETIMEDOUT/... plus "socket hang up"/"premature close" message patterns), retrying within the existing attempt/timeout budget. Thrift and CloudFetch body reads were moved inside the retried block so mid-stream "Premature close" is retried too. ExecuteStatement and other write-y methods keep NullRetryPolicy (no double-execution risk). 2. Fix CloudFetch prefetch-rejection leak. fetchNext fans out up to cloudFetchConcurrentDownloads downloads but only awaits the head; if the head rejects and the consumer stops iterating, the remaining in-flight promises surfaced as unhandledRejection. Each download is now reflected into a SettledDownload value at creation time, so a prefetched download can never become an unhandled rejection. The error is re-thrown only when the task is consumed, so observable behavior is unchanged. Testing: - HttpRetryPolicy unit suite 14/14 (incl. real ECONNRESET / "Premature close"). - Standalone checks against the built dist: zero leaked rejections when downloads fail and the consumer bails; a real node-fetch ECONNRESET against a local RST-injecting server recovers transparently (recovers on attempt 3). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…ckoff + operation-status fields (#420) * [SEA-NodeJS] Kernel-parity batch: mTLS + custom headers/UA + retry/backoff + operation-status fields Squashed rebase of the SEA kernel-parity batch onto current main (replaces the prior multi-commit branch, which had diverged with unsigned commits and a merge-commit history that conflicted with the since-merged #424/#426). Net feature set unchanged: - mTLS (clientCertPem/clientKeyPem) + custom HTTP headers + User-Agent entry on the SEA path, with PEM and header-token (CR/LF/NUL) validation in SeaAuth. - Retry/backoff tuning forwarded to the kernel (omitting unset knobs). - Operation-status fields surfaced through op.status(): numModifiedRows, displayMessage, diagnosticInfo, errorDetailsJson (async + sync paths), memoized once terminal; Thrift wire synthesis maps the same fields. - preserveBigNumericPrecision connection option (Thrift + SEA) and DATE/large-number parameter type-inference fix. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com> * fix(sea): address review findings + bump KERNEL_REV to kernel main; prettier Review findings (GopalDB / code-review agent), each validated: 1. DBSQLParameter DATE off-by-one in non-UTC timezones — `toISOString()` converts to UTC before slicing, so a local-constructed Date shifts a day in offset zones. Use local calendar accessors (getFullYear/getMonth/getDate). Validated end-to-end on a live SEA warehouse under TZ=Australia/Sydney: a `new Date(2024,2,14)` now round-trips as 2024-03-14 (was 2024-03-13). Regression test sabotages toISOString so it guards in any timezone (incl. the UTC CI runner). 2. SeaOperationBackend.status() (sync path) could block indefinitely — it gated Succeeded on `fetchHandlePromise` *existing*, but that is assigned synchronously when getFetchHandle() is first called while result() may still be pending; status() would then report Succeeded early AND await the pending result() via readRichStatusFields(). Gate on `blockingStatement` (set only after result() resolves) instead. 3. Async-path progress callback omitted the rich-status fields on the terminal Succeeded tick (the sync path includes them) — parity break for DML numModifiedRows. Merge the rich fields into the Succeeded callback in waitUntilReadyAsync. Also: - KERNEL_REV -> 9c2e2378 (kernel main, #144 merged). Binding contract (native/sea/index.d.ts) already matched the bumped surface. - prettier --write (lint fix) + new regression tests for findings 1-3. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com> --------- Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )